-
Notifications
You must be signed in to change notification settings - Fork 473
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Vitis 6327 Add PS kernel xclbins into APU Package #7594
Conversation
Signed-off-by: Daniel Benusovich <[email protected]>
…on yet Signed-off-by: Daniel Benusovich <[email protected]>
Signed-off-by: Daniel Benusovich <[email protected]>
Signed-off-by: Daniel Benusovich <[email protected]>
…. pkgapu not able to create xclbins due to missing symbols in ps kernel libraries Signed-off-by: Daniel Benusovich <[email protected]>
Signed-off-by: Daniel Benusovich <[email protected]>
…nto VITIS-6327
Signed-off-by: Daniel Benusovich <[email protected]>
Signed-off-by: Daniel Benusovich <[email protected]>
Signed-off-by: Daniel Benusovich <[email protected]>
Signed-off-by: Daniel Benusovich <[email protected]>
Signed-off-by: Daniel Benusovich <[email protected]>
Signed-off-by: Daniel Benusovich <[email protected]>
Build Passed! |
retest this please. |
Build failed :( |
…build script to use new file. Update apu package script to use new file Signed-off-by: Daniel Benusovich <[email protected]>
Build Passed! |
src/runtime_src/core/edge/ps_kernels/xrt/tests/validate/ps_aie_test/CMakeLists.txt
Outdated
Show resolved
Hide resolved
Signed-off-by: Daniel Benusovich <[email protected]>
Build Passed! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall this this looks good.
Regarding the validation tests, it would be great if future PRs has more comments in the cpp source code to assist the next developer in debugging issues.
src/runtime_src/core/edge/ps_kernels/xrt/tests/validate/ps_bandwidth_test/ps_bandwidth.cpp
Outdated
Show resolved
Hide resolved
retest this please |
Build failed :( |
retest this please. |
Build failed :( |
retest this please. |
Build Passed! |
Signed-off-by: Daniel Benusovich <[email protected]>
src/runtime_src/core/edge/ps_kernels/xrt/tests/validate/ps_bandwidth_test/ps_bandwidth.cpp
Show resolved
Hide resolved
Signed-off-by: Daniel Benusovich <[email protected]>
Build Passed! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should move away from sk_types.h. However, that should be taken up as a separate effort.
Problem solved by the commit
https://jira.xilinx.com/browse/VITIS-6327
The platform team requested that XRT create the xclbins that contain the PS kernels. They will generate the xclbins that contain the PL portions. This PR addresses adding the PS kernel xclbins into the APU package.
Bug / issue (if any) fixed, which PR introduced the bug, how it was discovered
The validation PS kernels were added into the build for edge platforms. The installation directory for all generated ps kernels is
/lib/firmware/xilinx/ps_kernels
. This is the same directory in which the apu files are stored.An additional json file is included to list out which PL xclbins are required to operate this PS kernel.
How problem was solved, alternative solutions (if any) and why they were rejected
Created CMake files that generate the Xclbins using a passed in reference to a VITIS directory. Added logic to install the generated xclbins into
/lib/firmware/xilinx/ps_kernels
withinpkgapu.sh
. xbutil will need to be updated in a 2nd PR.Risks (if any) associated the changes in the commit
Only one APU package can be installed at a time. If the time comes where multiple APU packages can be installed we must revist how the PS kernels are packaged.
Additionally, I am concerned about sourcing an internal directory within the yocto build to get access to
xclbinutil
. It would be better to allow this to be set some other way so that non-internal customers can build the apu package.What has been tested and how, request additional testing if necessary
Ubuntu 20.04
Documentation impact (if any)
None.